- 
                Notifications
    
You must be signed in to change notification settings  - Fork 1.2k
 
[new release] ppx_expect_nobase (0.17.3.0) #28790
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[new release] ppx_expect_nobase (0.17.3.0) #28790
Conversation
5c78efd    to
    3b53cfb      
    Compare
  
    There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks!
At first I was puzzled by the version-scheme 0.17.3+nobase-2, but then realized that you are aiming for agreement with the ppx_expect versions.
On that note, I noticed that you named this packages/ppx_expect_nobase/ppx_expect_nobase.0.17.3+nobase-2/opam
whereas the previous version was named
packages/ppx_expect_nobase/ppx_expect_nobase.v0.17.2.2/opam
- This drops the 
ppx_expect-compatiblev-prefix though. Was that an oversight? - Why switch from a 
.2suffix to a+nobase-2suffix? After all, the package is already namednobase, so that suffix-part seems redundant 🤔 
Unrelated: Would you consider an x-maintenance-intent entry?
https://github.com/ocaml/opam-repository/blob/master/governance/policies/archiving.md
3b53cfb    to
    0067233      
    Compare
  
    CHANGES: Strip base and other dependecies.
0067233    to
    801e062      
    Compare
  
    Signed-off-by: Kakadu <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks - all green - this is shaping up nicely! 😃
I have a couple of small questions/suggestions, but otherwise LGTM.
| bug-reports: "https://github.com/Kakadu/ppx_expect_nobase/issues" | ||
| depends: [ | ||
| "dune" {>= "3.11"} | ||
| "ocaml" {>= "4.14.2" & < "5.0.0" | >= "5.3.0" & <= "5.4.0"} | 
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just wondering: These are rather specific bounds.
- How does this fail on 5.0, 5.1, 5.2?
 - There's also a policy to avoid a preventive upper bound such as 
<= "5.4.0"
https://github.com/ocaml/opam-repository/tree/master/governance/policies#5-strict-dependency-constraints-should-be-avoided---or-preventive-upper-bounds 
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I removed the upper bound.
Signed-off-by: Kakadu <[email protected]>
| 
           Thanks  | 
    
Cram like framework for OCaml (with stripped dependencies)